feat(libexec): bklibcvenv — extract glibc wrapper-script pattern from pantry@5354c73f#348
Open
tannevaled wants to merge 2 commits into
Open
feat(libexec): bklibcvenv — extract glibc wrapper-script pattern from pantry@5354c73f#348tannevaled wants to merge 2 commits into
tannevaled wants to merge 2 commits into
Conversation
Extracts the recipe-side hermetic libc-bundling pattern @jhheider just validated inline in pkgxdev/pantry@5354c73f. Recipes that ship their own libc (glibc today; musl, custom libcs tomorrow) get a ~one-line call: - bklibcvenv seal {{prefix}} glibc-{{version.marketing}} instead of the ~20-line inline wrapper-installer that's currently duplicated in the glibc recipe. ## Shape Mirror of libexec/bkpyvenv (Python venv → relocatable stubs): libexec/bklibcvenv — the helper script share/brewkit/libcvenv-wrapper.sh — the POSIX sh wrapper template The helper: 1. takes `<prefix> <libdir-name>` as args 2. auto-detects LDSO from `uname -m` (overridable via LDSO env) 3. for each ELF in <prefix>/{bin,sbin}/* with a PT_INTERP: - moves it to <prefix>/libexec/<libc-name>-<dir>/ - replaces it with a sed-templated sh wrapper 4. wrapper does `cd $(dirname $0)/..` to compute prefix at runtime, then `exec $libdir/$ldso --library-path $libdir $libexec_path "$@"` `<libc-name>` (libdir-name up to first '-') prefixes the libexec subdir so multiple libc bottles can coexist (glibc + musl): - libexec/glibc-bin/ - libexec/glibc-sbin/ ## Why this is useful - **Fully relocatable** — bottle extracted at `/opt`, `~/.pkgx`, or any other path works the same; no absolute paths baked in. - **Sidesteps PT_INTERP** — invoking ld.so explicitly via wrapper ignores the ELF's PT_INTERP. brewkit's fix-elf currently can't rewrite PT_INTERP anyway (pkgxdev#345) and the bottle's own ld.so suffers from fix-elf's RPATH pollution when not `build.skip: fix-patchelf`'d. - **Reusable** — same shape works for musl, dietlibc, or any alternate libc anyone packages. ## Connection to bkwinvenv (pkgxdev#347) The Windows analog (`bkwinvenv`, sketched at pkgxdev#347) uses the same seal-into-libexec + emit-wrapper-script structure, just with `.cmd` wrappers + Windows DLL search-order semantics (no explicit loader invocation; co-location with the inner .exe is enough). Validating this Linux pattern empirically (which @jhheider's commit does) validates the Windows design too. ## Migration Once this lands, pkgxdev/pantry's glibc recipe can shrink its inline wrapper-installer block (currently ~20 lines + a prop: template) to one line. Same for any future libc recipe. Refs: pkgxdev#344 (this RFC), pkgxdev#347 (bkwinvenv sibling), pkgxdev/pantry@5354c73f (origin of pattern).
Author
|
Proposed glibc-recipe migration delta (for review purposes — not a PR yet). After this PR lands, - # PT_INTERP is absolute and cannot use $ORIGIN or fall back. Do not
- # bake {{prefix}} or its post-rename variant into installed binaries:
- # that breaks when the bottle relocates to ~/.pkgx. Instead, seal the
- # glibc-owned ELF tools into libexec and replace bin/* / sbin/* with
- # POSIX sh wrappers that invoke this bottle's ld.so by relative path.
- #
- # Running an ELF through ld.so explicitly ignores the ELF's own
- # PT_INTERP, so the inner libexec binaries may keep whatever glibc's
- # build installed. The wrapper supplies --library-path explicitly.
- - run: |
- for dir in bin sbin; do
- mkdir -p "{{prefix}}/libexec/glibc-$dir"
- for f in "{{prefix}}/$dir"/*; do
- [ -f "$f" ] && [ ! -L "$f" ] || continue
- patchelf --print-interpreter "$f" >/dev/null 2>&1 || continue
- mv "$f" "{{prefix}}/libexec/glibc-$dir"
- sed -e "s/@LDSO@/$LDSO/g" -e "s/@DIR@/$dir/g" "$PROP" >$f
- chmod 775 $f
- echo "wrapped $f"
- done
- done
- prop: |
- #!/bin/sh
-
- case "$0" in
- */*) bindir=${0%/*} ;;
- *) bindir=$(command -v -- "$0"); bindir=${bindir%/*} ;;
- esac
-
- prefix=$(CDPATH= cd -- "$bindir/.." && pwd)
-
- libdir="$prefix/lib/glibc-{{version.marketing}}"
-
- exec "$libdir/@LDSO@" --library-path "$libdir" "$prefix/libexec/glibc-@DIR@/$(basename "$0")" "$@"
+ # Seal bin/* + sbin/* into libexec/glibc-{bin,sbin}/ and replace
+ # them with relocatable POSIX sh wrappers that route through this
+ # bottle's ld.so. See pkgxdev/brewkit#348.
+ - bklibcvenv seal {{prefix}} glibc-{{version.marketing}}The other recipe edits in @jhheider's 5354c73f stay as-is (the linker-script-basename sed; the LDSO env var which is still used by the Net delta on glibc/package.yml: ~28 lines removed, 1 line added. If you'd rather I open the migration as a stacked pantry PR, happy to — but its CI would stay red until this brewkit PR lands, so it'd be noise. Easier to merge in lockstep: this PR + a 1-commit pantry PR side-by-side. |
Author
|
Extra evidence the inline shape works (@jhheider's CD run on So the relocatable wrapper-script approach works in production across 3 Linux distros + ARM64 + the buster-slim test sandbox (which was where everything was breaking earlier). That's the empirical case for the pattern; extracting it here as a brewkit-provided helper is just removing duplication. This PR is ready for review whenever you have a moment. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a bklibcvenv helper and wrapper template to make bundled libc bottles relocatable by routing packaged ELF binaries through the bottle’s ld.so via relative paths.
Changes:
- Introduce
bklibcvenv sealto wrap ELF binaries inbin/andsbin/using a wrapper template. - Add a POSIX-sh wrapper template that resolves
prefixat runtime and execs the bundled loader with--library-path. - Auto-detect loader name (
LDSO) based onuname -m, with env override.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| share/brewkit/libcvenv-wrapper.sh | Adds the runtime wrapper template used to exec binaries through the bundled ld.so. |
| libexec/bklibcvenv | Adds the sealing tool that moves ELF binaries to libexec/ and generates wrappers from the template. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+38
to
+39
| CMD=$1; shift | ||
|
|
| exit 2 | ||
| fi | ||
|
|
||
| set -x |
| -e "s|@LIBC_NAME@|$LIBC_NAME|g" \ | ||
| -e "s|@DIR@|$dir|g" \ | ||
| "$TEMPLATE" > "$f" | ||
| chmod 775 "$f" |
Comment on lines
+12
to
+15
| *) bindir=$(command -v -- "$0"); bindir=${bindir%/*} ;; | ||
| esac | ||
|
|
||
| prefix=$(CDPATH= cd -- "$bindir/.." && pwd) |
Comment on lines
+88
to
+93
| sed \ | ||
| -e "s|@LDSO@|$LDSO|g" \ | ||
| -e "s|@LIBDIR@|$LIBDIR_NAME|g" \ | ||
| -e "s|@LIBC_NAME@|$LIBC_NAME|g" \ | ||
| -e "s|@DIR@|$dir|g" \ | ||
| "$TEMPLATE" > "$f" |
All 5 points were legit: 1. **shift on empty args** (libexec/bklibcvenv:39) — with set -eo pipefail, `CMD=$1; shift` exited before the usage-error branch could fire when called with zero args. Now: check `$#` first. 2. **set -x unconditional** (line 73) — floods CI output. Gated behind BKLIBCVENV_DEBUG env var. 3. **chmod 775** (line 94) — group-writable on packaged artefacts is a bad-practice flag for security/packaging audits. Changed to 755. 4. **POSIX `--` in command -v / cd** (template line 12, 15) — not specified by POSIX for these built-ins; some old /bin/sh implementations reject. Dropped `--`; in our context $0 is never user-controlled so the defense was moot. 5. **sed-replacement metacharacter unsafety** (line 93) — values containing `&`, `\`, or the `|` delimiter would corrupt sed output. Defense in depth: added a `sed_escape()` helper and applied to all four interpolated values before substitution. Values in question (LDSO, LIBDIR_NAME, LIBC_NAME, dir) are all auto-detected or recipe-passed and won't realistically contain sed metachars, but Copilot's right that this should not be a latent footgun.
Author
|
Addressed all 5 Copilot review comments in 9ed9496:
Values in question are tightly controlled ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #344.
What
Extracts the wrapper-script pattern @jhheider just validated inline in pkgxdev/pantry@5354c73f (the post-merge follow-up on the glibc PR) into a reusable brewkit helper.
Mirrors the shape of
libexec/bkpyvenv:libexec/bklibcvenv— the recipe-side helpershare/brewkit/libcvenv-wrapper.sh— the POSIX sh wrapper templateRecipe-side API
vs. the current inline shape (~20 lines of for-loop + sed + chmod + an embedded
prop:template).Behaviour
For each ELF in
<prefix>/{bin,sbin}/*with a PT_INTERP:<prefix>/libexec/<libc>-<dir>/(where<libc>= "glibc" for glibc-2.43, "musl" for musl-1.2.x, etc.)<prefix>/<dir>/<name>with a generated POSIX sh wrapperGenerated wrapper computes its own bindir + climbs to prefix at runtime, then
exec'sld.so --library-path … <libexec-path> "$@". Fully relocatable — works at/opt,~/.pkgx, anywhere.LDSO is auto-detected from
uname -m(overridable viaLDSOenv var for exotic loaders).Why this matters
bkwinvenvwould have the exact same shape, just.cmdwrappers + Windows DLL search-order.bklibcvenv sealto bundle.Migration plan
Once this lands, the inline block in pkgxdev/pantry's glibc recipe (currently in commit @5354c73f) becomes a one-liner. Same for any future libc bottle.
Open questions
bklibcvenv stage? —bkpyvenvhas bothstageandseal. For libc, there's no equivalent of "create a venv" at build time — the recipe just builds libc normally; the only post-step isseal. So I left outstage. Worth adding for symmetry?<libdir-name>? — recipe passes it explicitly today; the helper could probably auto-detect from the onlyglibc-X.Y/dir underlib/. Brittle if multiple bottles ever coexist there.CDPATH= cd -- "$bindir/.."; the--is bash-friendly but POSIX-ambiguous on some old shells. Open to a tighter form.Testing
Not yet validated end-to-end (no CI run on this PR). The intent is to replace the inline block in pkgxdev/pantry's glibc recipe with
bklibcvenv seal …and confirm CI still passes — that's the natural test.Refs: #344 (RFC this closes), #346 (Windows port RFC — same pattern), #347 (
bkwinvenvsketch — Windows analog).